Skip to content

fix(app-check): type def fixes #8367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
mikehardy merged 8 commits into from
Apr 14, 2025
Merged

fix(app-check): type def fixes #8367

mikehardy merged 8 commits into from
Apr 14, 2025

Conversation

@MichaelVerdon
Copy link
Collaborator

@MichaelVerdon MichaelVerdon commented Feb 24, 2025 •
edited
Loading

Description

https://firebase.google.com/docs/reference/js/app-check.md#gettoken_39fc1b3

closes #8361

Related issues

Release Summary

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
    • Other (macOS, web)
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

Think react-native-firebase is great? Please consider supporting the project with any of the below:

@vercel
Copy link

vercel bot commented Feb 24, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 14, 2025 1:45pm

@MichaelVerdon MichaelVerdon marked this pull request as draft March 10, 2025 16:46
@russellwheatley
Copy link
Member

russellwheatley commented Mar 11, 2025

@MichaelVerdon - there are e2e tests failing because of the change to return type of getAppCheck() :https://github.com/invertase/react-native-firebase/actions/runs/13769712997/job/38560776497?pr=8367#step:16:14532

You also need to change these the way these are called:
https://github.com/invertase/react-native-firebase/blob/main/packages/app-check/lib/modular/index.js#L67
https://github.com/invertase/react-native-firebase/blob/main/packages/app-check/lib/modular/index.js#L77
https://github.com/invertase/react-native-firebase/blob/main/packages/app-check/lib/modular/index.js#L96

@MichaelVerdon MichaelVerdon marked this pull request as ready for review April 14, 2025 13:59
mikehardy
mikehardy approved these changes Apr 14, 2025
View reviewed changes
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pulled this locally for testing - and even after rebasing against current main, android and iOS appear to work fine for me, so it doesn't seem to have any functional issues. Code looks good on review, so LGTM for merge

@mikehardy mikehardy merged commit ecb1340 into main Apr 14, 2025
18 checks passed
@mikehardy mikehardy deleted the app-check-type-defs branch April 14, 2025 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mikehardy mikehardy mikehardy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[🐛] app-check modular API doesn't seem to conform to provided docs

4 participants

@MichaelVerdon @russellwheatley @mikehardy